-
Notifications
You must be signed in to change notification settings - Fork 1.3k
kvm: honour disk controller for vm during attach volume #12452
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
kvm: honour disk controller for vm during attach volume #12452
Conversation
Fixes apache#12090 Some years back apache#4569 allowed honouring dataDiskController for KVM VMs during start but the same behviour is not seen while attaching a volume to a running VM. This PR first checks in the AttachCommand if a valid controller is present. If a valid controller/bus type is not present than existing logic is used. Signed-off-by: Abhishek Kumar <[email protected]>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 4.20 #12452 +/- ##
=========================================
Coverage 16.23% 16.23%
- Complexity 13380 13381 +1
=========================================
Files 5657 5657
Lines 499039 499056 +17
Branches 60567 60569 +2
=========================================
+ Hits 81024 81032 +8
- Misses 408978 408986 +8
- Partials 9037 9038 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
DaanHoogland
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clgtm
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
Outdated
Show resolved
Hide resolved
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16399 |
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
Outdated
Show resolved
Hide resolved
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
Outdated
Show resolved
Hide resolved
|
@blueorangutan package |
|
@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16438 |
kiranchavala
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Followed the steps mentioned here
After adding the sixth disk , it was correctly assigned scsi driver
</disk>
<disk type='file' device='disk'>
<driver name='qemu' type='qcow2' cache='none'/>
<source file='/mnt/9dde54d6-ebe4-3f4e-b185-c88bf7419bfd/bd33ea5a-df96-4449-9b12-f58c1ff4a9ac' index='6'/>
<backingStore/>
<target dev='sdb' bus='scsi'/>
<serial>bd33ea5adf9644499b12</serial>
<alias name='scsi0-0-0-1'/>
<address type='drive' controller='0' bus='0' target='0' unit='1'/>
</disk>
<disk type='file' device='disk'>
<driver name='qemu' type='qcow2' cache='none'/>
<source file='/mnt/9dde54d6-ebe4-3f4e-b185-c88bf7419bfd/2837aa76-1adb-4d0b-b287-b8eabb8ece77' index='5'/>
<backingStore/>
<target dev='sdc' bus='scsi'/>
<serial>2837aa761adb4d0bb287</serial>
<alias name='scsi0-0-0-2'/>
<address type='drive' controller='0' bus='0' target='0' unit='2'/>
</disk>
<disk type='file' device='disk'>
<driver name='qemu' type='qcow2' cache='none'/>
<source file='/mnt/9dde54d6-ebe4-3f4e-b185-c88bf7419bfd/a3ccfebd-b3f6-4a23-975f-6fc178cd54d4' index='4'/>
<backingStore/>
<target dev='sde' bus='scsi'/>
<serial>a3ccfebdb3f64a23975f</serial>
<alias name='scsi0-0-0-4'/>
<address type='drive' controller='0' bus='0' target='0' unit='4'/>
</disk>
<disk type='file' device='disk'>
<driver name='qemu' type='qcow2' cache='none'/>
<source file='/mnt/734738cb-cf21-3b12-aa39-f007af1044ea/64f2bfa6-a7c3-4d44-a195-5eda2be1e59b' index='3'/>
<backingStore/>
<target dev='sdf' bus='scsi'/>
<serial>64f2bfa6a7c34d44a195</serial>
<alias name='scsi0-0-0-5'/>
<address type='drive' controller='0' bus='0' target='0' unit='5'/>
</disk>
<disk type='file' device='disk'>
<driver name='qemu' type='qcow2' cache='none'/>
<source file='/mnt/9dde54d6-ebe4-3f4e-b185-c88bf7419bfd/9007dd03-6806-410c-8c1e-94ac0e2e4e28' index='2'/>
<backingStore/>
<target dev='sdg' bus='scsi'/>
<serial>9007dd036806410c8c1e</serial>
<alias name='scsi0-0-0-6'/>
<address type='drive' controller='0' bus='0' target='0' unit='6'/>
</disk>
<disk type='file' device='disk'>
<driver name='qemu' type='qcow2' cache='none' discard='unmap'/>
<source file='/mnt/9dde54d6-ebe4-3f4e-b185-c88bf7419bfd/97b78eb6-7be1-4081-a52d-faa7c8056ee7' index='9'/>
<backingStore/>
<target dev='sdh' bus='scsi'/>
<serial>97b78eb67be14081a52d</serial>
<alias name='scsi1-0-0-0'/>
<address type='drive' controller='1' bus='0' target='0' unit='0'/>
</disk>
|
this is ready for merge, @shwstppr ? |
|
@DaanHoogland I've marked it ready though I'm not completely sure if this would solve the original issue reported in #12090 but definitely solves Kiran's case |
|
ok, will not close the issue (yet) but doesn’t hurt to merge than. |
Description
Fixes #12090
Some years back #4569 allowed honouring dataDiskController for KVM VMs during start but the same behviour is not seen while attaching a volume to a running VM.
This PR first checks in the AttachCommand if a valid controller is present. If a valid controller/bus type is not present than existing logic is used.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?